Skip to content

Conversation

Jack251970
Copy link
Member

Use DialogJump to get explorer path

Follow on with #1018.

If we use DialogJump to get explorer path, it can use the third-party explorer plugins and also support to get the path from the most recent tab.

@Jack251970 Jack251970 enabled auto-merge September 6, 2025 07:55
@Jack251970 Jack251970 requested a review from Copilot September 6, 2025 07:55
@Jack251970 Jack251970 added this to the Future milestone Sep 6, 2025
@Jack251970 Jack251970 added the enhancement New feature or request label Sep 6, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR simplifies the FileExplorerHelper implementation by replacing custom Windows explorer detection logic with the DialogJump library. This change enables support for third-party file explorer plugins and allows getting paths from the most recent active tab.

  • Replaces complex COM-based Windows explorer detection with DialogJump.GetActiveExplorerPath()
  • Removes unused imports and over 70 lines of custom z-order and window enumeration code
  • Improves string comparison efficiency by using char literal instead of string

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

gitstream-cm bot commented Sep 6, 2025

🥷 Code experts: no user but you matched threshold 10

Jack251970 has most 🧠 knowledge in the files.

See details

Flow.Launcher.Infrastructure/FileExplorerHelper.cs

Knowledge based on git-blame:
Jack251970: 18%

✨ Comment /gs review for LinearB AI review. Learn how to automate it here.

Copy link

gitstream-cm bot commented Sep 6, 2025

Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX.

Copy link
Contributor

coderabbitai bot commented Sep 6, 2025

📝 Walkthrough

Walkthrough

Refactors FileExplorerHelper to obtain the active File Explorer path via DialogJump.DialogJump.GetActiveExplorerPath(), converting the returned URI to a local path before normalization. Removes COM/window z-order enumeration helpers and unused usings. Adjusts trailing backslash handling to use a char literal. Public method signatures remain unchanged.

Changes

Cohort / File(s) Change summary
File Explorer path retrieval refactor
Flow.Launcher.Infrastructure/FileExplorerHelper.cs
Replaced COM/window enumeration with DialogJump-based path retrieval; convert URI to local path and pass to GetDirectoryPath; GetDirectoryPath uses EndsWith('\') check; removed GetActiveExplorer/GetZOrder and related usings; retained public method signatures and null/empty handling.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant App as Caller
    participant FEH as FileExplorerHelper
    participant DJ as DialogJump.DialogJump

    User->>App: Trigger "GetActiveExplorerPath"
    App->>FEH: GetActiveExplorerPath()
    FEH->>DJ: GetActiveExplorerPath()
    DJ-->>FEH: uriString or null
    alt uriString present
        FEH->>FEH: Convert to Uri → LocalPath
        FEH->>FEH: GetDirectoryPath(localPath)\n(ensure trailing '\\')
        FEH-->>App: normalizedPath
    else null/empty
        FEH-->>App: null
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • taooceros
  • jjw24
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch FileExplorerHelper_enhancement

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
Flow.Launcher.Infrastructure/FileExplorerHelper.cs (1)

23-29: Use Path helpers for separator handling (more robust, self-documenting).

Handles both separators and aligns with platform constants.

Apply this diff within the selected lines:

-            if (!path.EndsWith('\\'))
-            {
-                return path + "\\";
-            }
+            if (!Path.EndsInDirectorySeparator(path))
+            {
+                return path + Path.DirectorySeparatorChar;
+            }

And add (outside the selected lines) at the top of the file:

+using System.IO;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b898c46 and f40255b.

📒 Files selected for processing (1)
  • Flow.Launcher.Infrastructure/FileExplorerHelper.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.

Applied to files:

  • Flow.Launcher.Infrastructure/FileExplorerHelper.cs
📚 Learning: 2025-09-04T11:52:29.074Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.074Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.

Applied to files:

  • Flow.Launcher.Infrastructure/FileExplorerHelper.cs
🧬 Code graph analysis (1)
Flow.Launcher.Infrastructure/FileExplorerHelper.cs (1)
Flow.Launcher.Infrastructure/DialogJump/DialogJump.cs (1)
  • DialogJump (19-1078)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: gitStream workflow automation
  • GitHub Check: gitStream.cm
  • GitHub Check: gitStream.cm
  • GitHub Check: gitStream.cm
  • GitHub Check: gitStream.cm
  • GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher.Infrastructure/FileExplorerHelper.cs (1)

12-15:

@github-actions github-actions bot modified the milestones: Future, 2.1.0 Sep 13, 2025
@Jack251970 Jack251970 merged commit 2dd4160 into dev Sep 18, 2025
14 of 15 checks passed
@Jack251970 Jack251970 deleted the FileExplorerHelper_enhancement branch September 18, 2025 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants